Fix Android QR scanner lifecycle and location fallback - #899
Conversation
a456f12 to
dcee3b4
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcee3b417b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CameraXViewfinder( | ||
| surfaceRequest = request, | ||
| implementationMode = ImplementationMode.EMBEDDED, | ||
| modifier = Modifier.fillMaxSize() |
There was a problem hiding this comment.
Keep ImplementationMode.EMBEDDED here because this preview is rendered inside the tab Crossfade and a rounded clipped container. Without the argument, CameraXViewfinder defaults to the external/SurfaceView implementation, which does not support the alpha blending and clipping this UI relies on; when users switch between Scan and My QR, the camera surface can remain opaque during the transition or render outside the rounded bounds.
Useful? React with 👍 / 👎.
Chessing234
left a comment
Chessing234
left a comment
There was a problem hiding this comment.
this is two changes in one PR. the QR scanner lifecycle work touches VerificationSheet.kt; the Fused-to-system location fallback touches FusedLocationProvider.kt and LocationNotesSheetPresenter.kt and has nothing to do with the scanner. they'll want reviewing by different eyes and they carry different risk — the location fallback changes where every geohash user's position comes from.
splitting them would also decouple you from #900, which is fixing the same camera-startup path in the same file and will conflict with the scanner half of this.
on the location half specifically: falling back to the system provider when Fused is unavailable is reasonable, but worth stating in the description what the accuracy and update-cadence difference is, since geohash channel selection is derived from it — a coarser fix silently putting someone in a neighbouring cell is the kind of thing that's hard to trace back here.
2 participants
Summary
Validation
:app:compileDebugKotlin --rerun-tasks:app:assembleDebug:app:lintDebug(the task completes; the repository has existing lint findings)Test limitation
:app:testDebugUnitTestis currently blocked in this checkout by an existing test-runner classpath issue: compiled test classes are not found by the runner.